From: Yehuda Katz <wycats@gmail.com>
Date: Tue, 6 May 2014 02:33:07 +0000 (-0700)
Subject: Break apart internal and CLI errors
X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~1089
X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/success//%22http:/www.example.com/cgi/success/?a=commitdiff_plain;h=50f110a4c00e65065f418ac904fc3d4b767a48f9;p=cargo.git

Break apart internal and CLI errors

The Cargo library now uses internal errors to represent problems, and
the operations convert those errors into CLIErrors, which contain a
user-friendly error message, additional information for verbose mode,
and an exit code.
---

diff --git a/src/bin/cargo-compile.rs b/src/bin/cargo-compile.rs
index 12ab5107e..0c2a5d613 100644
--- a/src/bin/cargo-compile.rs
+++ b/src/bin/cargo-compile.rs
@@ -6,7 +6,7 @@ extern crate hammer;
 extern crate serialize;
 
 use cargo::ops::cargo_compile::compile;
-use cargo::{CargoResult,ToCargoError};
+use cargo::core::errors::{CLIResult,CLIError,ToResult};
 use hammer::{FlagDecoder,FlagConfig,HammerError};
 use serialize::Decodable;
 
@@ -17,18 +17,19 @@ pub struct Options {
 
 impl FlagConfig for Options {}
 
-fn flags<T: FlagConfig + Decodable<FlagDecoder, HammerError>>() -> CargoResult<T> {
+fn flags<T: FlagConfig + Decodable<FlagDecoder, HammerError>>() -> CLIResult<T> {
     let mut decoder = FlagDecoder::new::<T>(std::os::args().tail());
-    Decodable::decode(&mut decoder).to_cargo_error(|e: HammerError| e.message, 1)
+    Decodable::decode(&mut decoder).to_result(|e: HammerError| CLIError::new(e.message, None, 1))
 }
 
-fn execute() -> CargoResult<()> {
-    compile(try!(flags::<Options>()).manifest_path.as_slice())
+fn execute() -> CLIResult<()> {
+    compile(try!(flags::<Options>()).manifest_path.as_slice()).to_result(|_|
+        CLIError::new("Compilation failed", None, 1))
 }
 
 fn main() {
     match execute() {
-        Err(io_error) => fail!("{}", io_error),
+        Err(err) => fail!("{}", err),
         Ok(_) => return
     }
 }
diff --git a/src/bin/cargo-read-manifest.rs b/src/bin/cargo-read-manifest.rs
index 5007e42e3..bb0ebfcc8 100644
--- a/src/bin/cargo-read-manifest.rs
+++ b/src/bin/cargo-read-manifest.rs
@@ -5,9 +5,10 @@ extern crate cargo;
 extern crate serialize;
 extern crate hammer;
 
-use cargo::{CargoResult,execute_main_without_stdin};
+use cargo::execute_main_without_stdin;
 use cargo::ops::cargo_read_manifest::read_manifest;
 use cargo::core::Manifest;
+use cargo::core::errors::CLIResult;
 use hammer::FlagConfig;
 
 #[deriving(Decodable,Eq,Clone,Ord)]
@@ -21,7 +22,7 @@ fn main() {
     execute_main_without_stdin(execute);
 }
 
-fn execute(flags: ReadManifestFlags) -> CargoResult<Option<Manifest>> {
+fn execute(flags: ReadManifestFlags) -> CLIResult<Option<Manifest>> {
     match read_manifest(flags.manifest_path) {
         Ok(manifest) => Ok(Some(manifest)),
         Err(e) => Err(e)
diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs
index 08b366b89..cca534dbb 100644
--- a/src/bin/cargo.rs
+++ b/src/bin/cargo.rs
@@ -7,7 +7,8 @@ extern crate collections;
 use hammer::{FlagConfig,FlagConfiguration};
 use std::os;
 use serialize::Encodable;
-use cargo::{CargoResult,ToCargoError,NoFlags,execute_main_without_stdin,handle_error};
+use cargo::{NoFlags,execute_main_without_stdin,handle_error};
+use cargo::core::errors::{CLIError,CLIResult,ToResult};
 use cargo::util::important_paths::find_project;
 use cargo::util::config;
 
@@ -36,9 +37,9 @@ fn execute() {
     else if cmd == "locate-project".to_owned() { execute_main_without_stdin(locate_project) }
 }
 
-fn process(mut args: ~[~str]) -> CargoResult<(~str, ~[~str])> {
+fn process(mut args: ~[~str]) -> CLIResult<(~str, ~[~str])> {
     args = args.tail().to_owned();
-    let head = try!(args.head().to_cargo_error("No subcommand found".to_owned(), 1)).to_owned();
+    let head = try!(args.head().to_result(|_| CLIError::new("No subcommand found", None, 1))).to_owned();
     let tail = args.tail().to_owned();
 
     Ok((head, tail))
@@ -61,8 +62,9 @@ impl FlagConfig for ConfigForKeyFlags {
     }
 }
 
-fn config_for_key(args: ConfigForKeyFlags) -> CargoResult<Option<ConfigOut>> {
-    let value = try!(config::get_config(os::getcwd(), args.key.as_slice()));
+fn config_for_key(args: ConfigForKeyFlags) -> CLIResult<Option<ConfigOut>> {
+    let value = try!(config::get_config(os::getcwd(), args.key.as_slice()).to_result(|err|
+        CLIError::new("Couldn't load configuration", Some(err.to_str()), 1)));
 
     if args.human {
         println!("{}", value);
@@ -85,8 +87,9 @@ impl FlagConfig for ConfigListFlags {
     }
 }
 
-fn config_list(args: ConfigListFlags) -> CargoResult<Option<ConfigOut>> {
-    let configs = try!(config::all_configs(os::getcwd()));
+fn config_list(args: ConfigListFlags) -> CLIResult<Option<ConfigOut>> {
+    let configs = try!(config::all_configs(os::getcwd()).to_result(|err|
+        CLIError::new("Couldn't load conifguration", Some(err.to_str()), 1)));
 
     if args.human {
         for (key, value) in configs.iter() {
@@ -98,8 +101,12 @@ fn config_list(args: ConfigListFlags) -> CargoResult<Option<ConfigOut>> {
     }
 }
 
-fn locate_project(_: NoFlags) -> CargoResult<Option<ProjectLocation>> {
-    let root = try!(find_project(os::getcwd(), "Cargo.toml".to_owned()));
-    let string = try!(root.as_str().to_cargo_error(format!("Your project path contains characters not representable in Unicode: {}", os::getcwd().display()), 1));
+fn locate_project(_: NoFlags) -> CLIResult<Option<ProjectLocation>> {
+    let root = try!(find_project(os::getcwd(), "Cargo.toml".to_owned()).to_result(|err|
+        CLIError::new(err.to_str(), None, 1)));
+
+    let string = try!(root.as_str().to_result(|_|
+        CLIError::new(format!("Your project path contains characters not representable in Unicode: {}", os::getcwd().display()), None, 1)));
+
     Ok(Some(ProjectLocation { root: string.to_owned() }))
 }
diff --git a/src/cargo/core/errors.rs b/src/cargo/core/errors.rs
new file mode 100644
index 000000000..25d5fb746
--- /dev/null
+++ b/src/cargo/core/errors.rs
@@ -0,0 +1,139 @@
+use std::fmt;
+use std::fmt::{Show,Formatter};
+use std::io::IoError;
+
+/**
+ * There are two kinds of errors returned by Cargo functions:
+ *
+ * * CargoCLIError, which represents a failure that can be directly presented
+ *   to the user.
+ * * CargoInternalError, which represents an internal failure that must be
+ *   converted into a CargoCLIError before it may be presented to the user.
+ *
+ * These two kinds of errors are wrapped up in a `CargoError` enum.
+ *
+ * Cargo functions must first convert all other kinds of errors (such as
+ * IoError) into one of the Cargo errors before returning.
+ *
+ * This module includes several convenience functions for working with these
+ * different kinds of errors:
+ *
+ * `to_result::<E1, E2>(|E1| -> E2) -> E2` converts any kind of error into
+ * another kind of error. It can be used together with `try!`, as in:
+ *
+ *     try!(mkdir(path).to_result(|err| {
+ *         let msg = format!("Couldn't make directory {}", path.display());
+ *         CargoError::cli(msg, 12)
+ *     })
+ *
+ * `to_result::<Option<T>, E>(|| -> E) -> E` converts a `None` value into
+ * another kind of error. It can also be used together with `try!`:
+ *
+ *     try!(from_str(val).to_result(|| {
+ *         CargoError::internal(StringConversionError(val))
+ *     })
+ */
+
+pub type CargoResult<T> = Result<T, CargoError>;
+pub type CLIResult<T> = Result<T, CLIError>;
+
+pub enum CargoError {
+    CargoInternalError(InternalError),
+    CargoCLIError(CLIError)
+}
+
+impl Show for CargoError {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        match self {
+            &CargoInternalError(ref err) => write!(f.buf, "{}", err),
+            &CargoCLIError(ref err) => write!(f.buf, "{}", err)
+        }
+    }
+}
+
+pub struct CLIError {
+    pub msg: ~str,
+    pub detail: Option<~str>,
+    pub exit_code: uint
+}
+
+impl CLIError {
+    pub fn new<T: ToStr>(msg: T, detail: Option<~str>, exit_code: uint) -> CLIError {
+        CLIError { msg: msg.to_str(), detail: detail, exit_code: exit_code }
+    }
+}
+
+impl Show for CLIError {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f.buf, "{}", self.msg)
+    }
+}
+
+pub enum InternalError {
+    StringConversionError(~str, &'static str),
+    MissingManifest(Path, ~str),
+    WrappedIoError(IoError),
+    PathError(~str),
+    Described(~str),
+    Other
+}
+
+impl Show for InternalError {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        match self {
+            &StringConversionError(ref string, ref type_name) => {
+                write!(f.buf, "Couldn't convert `{}` into {}", string, type_name)
+            },
+            &MissingManifest(ref path, ref file) => {
+                write!(f.buf, "Couldn't find a {} in the project (`{}` or any parent directory", file, path.display())
+            },
+            &WrappedIoError(ref io_error) => {
+                write!(f.buf, "{}", io_error)
+            },
+            &PathError(ref s) | &Described(ref s) => {
+                write!(f.buf, "{}", s)
+            },
+            &Other => write!(f.buf, "Other internal error")
+        }
+    }
+}
+
+impl CargoError {
+    pub fn cli(msg: ~str, detail: Option<~str>, exit_code: uint) -> CargoError {
+        CargoCLIError(CLIError::new(msg, detail, exit_code))
+    }
+
+    pub fn internal(error: InternalError) -> CargoError {
+        CargoInternalError(error)
+    }
+
+    pub fn cli_error(self) -> CLIError {
+        match self {
+            CargoInternalError(err) =>
+                CLIError::new("An unexpected error occurred", Some(err.to_str()), 100),
+            CargoCLIError(err) => err
+        }
+    }
+}
+
+pub trait ToResult<T,E1,E2> {
+    fn to_result(self, callback: |E1| -> E2) -> Result<T,E2>;
+}
+
+impl<T,E1,E2> ToResult<T,E1,E2> for Result<T,E1> {
+    fn to_result(self, callback: |E1| -> E2) -> Result<T,E2> {
+        match self {
+            Ok(val) => Ok(val),
+            Err(e) => Err(callback(e))
+        }
+    }
+}
+
+impl<T,E> ToResult<T,Option<T>,E> for Option<T> {
+    fn to_result(self, callback: |Option<T>| -> E) -> Result<T,E> {
+        match self {
+            Some(val) => Ok(val),
+            None => Err(callback(self))
+        }
+    }
+}
diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs
index 67660a229..92a909f81 100644
--- a/src/cargo/core/manifest.rs
+++ b/src/cargo/core/manifest.rs
@@ -1,7 +1,7 @@
 use core::NameVer;
 use core::dependency::Dependency;
 use collections::HashMap;
-use {CargoResult,ToCargoError};
+use core::errors::{CargoResult,CargoError,ToResult,PathError};
 
 /*
  * TODO: Make all struct fields private
@@ -47,8 +47,11 @@ impl Manifest {
             }).collect()
         }).unwrap_or_else(|| vec!());
 
+        let root = try!(Path::new(path.to_owned()).dirname_str().map(|s| s.to_owned()).to_result(|_|
+            CargoError::internal(PathError(format!("Couldn't convert {} to a directory name", path)))));
+
         Ok(Manifest {
-            root: try!(Path::new(path.to_owned()).dirname_str().to_cargo_error(format!("Could not get dirname from {}", path), 1)).to_owned(),
+            root: root.to_owned(),
             project: project.clone(),
             lib: lib,
             bin: bin,
diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs
index d9ea34eb7..14d108d17 100644
--- a/src/cargo/core/mod.rs
+++ b/src/cargo/core/mod.rs
@@ -20,6 +20,7 @@ pub use self::package::{
 
 pub use self::dependency::Dependency;
 
+pub mod errors;
 pub mod namever;
 pub mod source;
 pub mod package;
diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs
index c4e22b102..7d22a718b 100644
--- a/src/cargo/core/resolver.rs
+++ b/src/cargo/core/resolver.rs
@@ -1,7 +1,7 @@
 use collections::HashMap;
 use core;
 use core::package::PackageSet;
-use {CargoResult};
+use core::errors::CargoResult;
 
 #[allow(dead_code)]
 pub fn resolve(deps: &[core::Dependency], registry: &core::Registry) -> CargoResult<PackageSet> {
diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs
index 78677f3ae..6132ce323 100644
--- a/src/cargo/core/source.rs
+++ b/src/cargo/core/source.rs
@@ -1,11 +1,12 @@
 use core::{NameVer,Package};
-use CargoResult;
+use core::errors::CargoResult;
+use std::fmt::Show;
 
 /**
  * A Source finds and downloads remote packages based on names and
  * versions.
  */
-pub trait Source {
+pub trait Source : Show {
     /**
      * The update method performs any network operations required to
      * get the entire list of all names, versions and dependencies of
diff --git a/src/cargo/mod.rs b/src/cargo/mod.rs
index 77653915f..b0ad6c23e 100644
--- a/src/cargo/mod.rs
+++ b/src/cargo/mod.rs
@@ -15,9 +15,8 @@ extern crate hamcrest;
 
 use serialize::{Decoder,Encoder,Decodable,Encodable,json};
 use std::io;
-use std::fmt;
-use std::fmt::{Show,Formatter};
 use hammer::{FlagDecoder,FlagConfig,HammerError};
+pub use core::errors::{CLIError,CLIResult,ToResult};
 
 macro_rules! some(
   ($e:expr) => (
@@ -32,64 +31,6 @@ pub mod ops;
 pub mod sources;
 pub mod util;
 
-
-pub type CargoResult<T> = Result<T, CargoError>;
-
-pub struct CargoError {
-    message: ~str,
-    exit_code: uint
-}
-
-impl CargoError {
-    pub fn new(message: ~str, exit_code: uint) -> CargoError {
-        CargoError { message: message, exit_code: exit_code }
-    }
-}
-
-impl Show for CargoError {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        write!(f.buf, "{}", self.message)
-    }
-}
-
-pub trait ToCargoErrorMessage<E> {
-    fn to_cargo_error_message(self, error: E) -> ~str;
-}
-
-impl<E> ToCargoErrorMessage<E> for ~str {
-    fn to_cargo_error_message(self, _: E) -> ~str {
-        self
-    }
-}
-
-impl<'a, E> ToCargoErrorMessage<E> for |E|:'a -> ~str {
-    fn to_cargo_error_message(self, err: E) -> ~str {
-        self(err)
-    }
-}
-
-pub trait ToCargoError<T, E> {
-    fn to_cargo_error<M: ToCargoErrorMessage<E>>(self, to_message: M, exit_code: uint) -> Result<T, CargoError>;
-}
-
-impl<T,E> ToCargoError<T, E> for Result<T,E> {
-    fn to_cargo_error<M: ToCargoErrorMessage<E>>(self, to_message: M, exit_code: uint) -> Result<T, CargoError> {
-        match self {
-            Err(err) => Err(CargoError{ message: to_message.to_cargo_error_message(err), exit_code: exit_code }),
-            Ok(val) => Ok(val)
-        }
-    }
-}
-
-impl<T> ToCargoError<T, Option<T>> for Option<T> {
-    fn to_cargo_error<M: ToCargoErrorMessage<Option<T>>>(self, to_message: M, exit_code: uint) -> Result<T, CargoError> {
-        match self {
-            None => Err(CargoError{ message: to_message.to_cargo_error_message(None), exit_code: exit_code }),
-            Some(val) => Ok(val)
-        }
-    }
-}
-
 trait RepresentsFlags : FlagConfig + Decodable<FlagDecoder, HammerError> {}
 impl<T: FlagConfig + Decodable<FlagDecoder, HammerError>> RepresentsFlags for T {}
 
@@ -101,8 +42,8 @@ pub struct NoFlags;
 
 impl FlagConfig for NoFlags {}
 
-pub fn execute_main<'a, T: RepresentsFlags, U: RepresentsJSON, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T, U) -> CargoResult<Option<V>>) {
-    fn call<'a, T: RepresentsFlags, U: RepresentsJSON, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T, U) -> CargoResult<Option<V>>) -> CargoResult<Option<V>> {
+pub fn execute_main<'a, T: RepresentsFlags, U: RepresentsJSON, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T, U) -> CLIResult<Option<V>>) {
+    fn call<'a, T: RepresentsFlags, U: RepresentsJSON, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T, U) -> CLIResult<Option<V>>) -> CLIResult<Option<V>> {
         let flags = try!(flags_from_args::<T>());
         let json = try!(json_from_stdin::<U>());
 
@@ -112,8 +53,8 @@ pub fn execute_main<'a, T: RepresentsFlags, U: RepresentsJSON, V: Encodable<json
     process_executed(call(exec))
 }
 
-pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T) -> CargoResult<Option<V>>) {
-    fn call<'a, T: RepresentsFlags, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T) -> CargoResult<Option<V>>) -> CargoResult<Option<V>> {
+pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T) -> CLIResult<Option<V>>) {
+    fn call<'a, T: RepresentsFlags, V: Encodable<json::Encoder<'a>, io::IoError>>(exec: fn(T) -> CLIResult<Option<V>>) -> CLIResult<Option<V>> {
         let flags = try!(flags_from_args::<T>());
 
         exec(flags)
@@ -122,7 +63,7 @@ pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable<json::Enc
     process_executed(call(exec))
 }
 
-pub fn process_executed<'a, T: Encodable<json::Encoder<'a>, io::IoError>>(result: CargoResult<Option<T>>) {
+pub fn process_executed<'a, T: Encodable<json::Encoder<'a>, io::IoError>>(result: CLIResult<Option<T>>) {
     match result {
         Err(e) => handle_error(e),
         Ok(encodable) => {
@@ -134,22 +75,22 @@ pub fn process_executed<'a, T: Encodable<json::Encoder<'a>, io::IoError>>(result
     }
 }
 
-pub fn handle_error(err: CargoError) {
-    let _ = write!(&mut std::io::stderr(), "{}", err.message);
+pub fn handle_error(err: CLIError) {
+    let _ = write!(&mut std::io::stderr(), "{}", err.msg);
     std::os::set_exit_status(err.exit_code as int);
 }
 
-fn flags_from_args<T: RepresentsFlags>() -> CargoResult<T> {
+fn flags_from_args<T: RepresentsFlags>() -> CLIResult<T> {
     let mut decoder = FlagDecoder::new::<T>(std::os::args().tail());
-    Decodable::decode(&mut decoder).to_cargo_error(|e: HammerError| e.message, 1)
+    Decodable::decode(&mut decoder).to_result(|e: HammerError| CLIError::new(e.message, None, 1))
 }
 
-fn json_from_stdin<T: RepresentsJSON>() -> CargoResult<T> {
+fn json_from_stdin<T: RepresentsJSON>() -> CLIResult<T> {
     let mut reader = io::stdin();
-    let input = try!(reader.read_to_str().to_cargo_error("Cannot read stdin to a string".to_owned(), 1));
+    let input = try!(reader.read_to_str().to_result(|_| CLIError::new("Standard in did not exist or was not UTF-8", None, 1)));
 
-    let json = try!(json::from_str(input).to_cargo_error(format!("Cannot parse json: {}", input), 1));
+    let json = try!(json::from_str(input).to_result(|_| CLIError::new("Could not parse standard in as JSON", Some(input.clone()), 1)));
     let mut decoder = json::Decoder::new(json);
 
-    Decodable::decode(&mut decoder).to_cargo_error(|e: json::DecoderError| format!("{}", e), 1)
+    Decodable::decode(&mut decoder).to_result(|e: json::DecoderError| CLIError::new("Could not process standard in as input", Some(e.to_str()), 1))
 }
diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs
index 9405acde3..c4b006931 100644
--- a/src/cargo/ops/cargo_compile.rs
+++ b/src/cargo/ops/cargo_compile.rs
@@ -24,29 +24,34 @@ use core::source::Source;
 use core::dependency::Dependency;
 use sources::path::PathSource;
 use ops::cargo_rustc;
-use {CargoError,CargoResult};
+use core::errors::{CargoError,CLIError,CLIResult,ToResult};
+
+pub fn compile(manifest_path: &str) -> CLIResult<()> {
+    let configs = try!(all_configs(os::getcwd()).to_result(|err: CargoError|
+        CLIError::new("Could not load configurations", Some(err.to_str()), 1)));
 
-pub fn compile(manifest_path: &str) -> CargoResult<()> {
-    let configs = try!(all_configs(os::getcwd()));
     let config_paths = configs.find(&("paths".to_owned())).map(|v| v.clone()).unwrap_or_else(|| ConfigValue::new());
 
     let paths = match config_paths.get_value() {
-        &config::String(_) => return Err(CargoError::new("The path was configured as a String instead of a List".to_owned(), 1)),
+        &config::String(_) => return Err(CLIError::new("The path was configured as a String instead of a List", None, 1)),
         &config::List(ref list) => list.iter().map(|path| Path::new(path.as_slice())).collect()
     };
 
     let source = PathSource::new(paths);
-    let names = try!(source.list());
-    try!(source.download(names.as_slice()));
+    let names = try!(source.list().to_result(|err| CLIError::new(format!("Unable to list packages from {}", source), Some(err.to_str()), 1)));
+    try!(source.download(names.as_slice()).to_result(|err| CLIError::new(format!("Unable to download packages from {}", source), Some(err.to_str()), 1)));
 
     let deps: Vec<Dependency> = names.iter().map(|namever| {
         Dependency::with_namever(namever)
     }).collect();
 
-    let packages = try!(source.get(names.as_slice()));
+    let packages = try!(source.get(names.as_slice()).to_result(|err|
+        CLIError::new(format!("Unable to get packages from {} for {}", source, names), Some(err.to_str()), 1)));
+
     let registry = PackageSet::new(packages.as_slice());
 
-    let resolved = try!(resolve(deps.as_slice(), &registry));
+    let resolved = try!(resolve(deps.as_slice(), &registry).to_result(|err: CargoError|
+        CLIError::new("Unable to resolve dependencies", Some(err.to_str()), 1)));
 
     try!(cargo_rustc::compile(&resolved));
 
diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs
index 0666c666b..264dd917a 100644
--- a/src/cargo/ops/cargo_read_manifest.rs
+++ b/src/cargo/ops/cargo_read_manifest.rs
@@ -1,11 +1,15 @@
 use toml;
 use toml::from_toml;
 use core::manifest::{SerializedManifest,Manifest};
-use {CargoResult,ToCargoError};
+use core::errors::{CLIError,CLIResult,ToResult};
 
-pub fn read_manifest(manifest_path: &str) -> CargoResult<Manifest> {
-    let root = try!(toml::parse_from_file(manifest_path.clone()).to_cargo_error(format!("Couldn't parse Toml file: {}", manifest_path), 1));
-    let toml_manifest = try!(from_toml::<SerializedManifest>(root.clone()).to_cargo_error(|e: toml::Error| format!("Couldn't parse Toml file: {:?}", e), 1));
+pub fn read_manifest(manifest_path: &str) -> CLIResult<Manifest> {
+    let root = try!(toml::parse_from_file(manifest_path.clone()).to_result(|err|
+        CLIError::new(format!("Cargo.toml was not valid Toml: {}", manifest_path), Some(err.to_str()), 1)));
 
-    Manifest::from_serialized(manifest_path.as_slice(), &toml_manifest)
+    let toml_manifest = try!(from_toml::<SerializedManifest>(root.clone()).to_result(|err: toml::Error|
+        CLIError::new(format!("Cargo.toml was not in the right format: {}", manifest_path), Some(err.to_str()), 1)));
+
+    Manifest::from_serialized(manifest_path.as_slice(), &toml_manifest).to_result(|err|
+        CLIError::new(format!("Cargo.toml was not in the right format: {}", manifest_path), Some(err.to_str()), 1))
 }
diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs
index 8216d6eb6..e4fc8fa3a 100644
--- a/src/cargo/ops/cargo_rustc.rs
+++ b/src/cargo/ops/cargo_rustc.rs
@@ -3,16 +3,17 @@ use std::os::args;
 use std::io;
 use std::io::process::{Process,ProcessConfig,InheritFd};
 use std::path::Path;
-use {CargoResult,CargoError,ToCargoError,NoFlags};
+use core::errors::{CLIError,CLIResult,ToResult};
+use NoFlags;
 use core;
 use util;
 
 type Args = Vec<~str>;
 
-pub fn compile(pkgs: &core::PackageSet) -> CargoResult<()> {
+pub fn compile(pkgs: &core::PackageSet) -> CLIResult<()> {
     let sorted = match pkgs.sort() {
         Some(pkgs) => pkgs,
-        None => return Err(CargoError::new("Circular dependency detected".to_owned(), 1))
+        None => return Err(CLIError::new("Circular dependency detected", None, 1))
     };
 
     for pkg in sorted.iter() {
@@ -23,13 +24,14 @@ pub fn compile(pkgs: &core::PackageSet) -> CargoResult<()> {
 }
 
 
-fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()> {
+fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CLIResult<()> {
     // Build up the destination
     let src = pkg.get_root().join(Path::new(pkg.get_source().path.as_slice()));
     let target = pkg.get_root().join(Path::new(pkg.get_target()));
 
     // First ensure that the directory exists
-    try!(mk_target(&target).to_cargo_error(format!("Could not create the target directory {}", target.display()), 1));
+    try!(mk_target(&target).to_result(|err|
+        CLIError::new(format!("Could not create the target directory {}", target.display()), Some(err.to_str()), 1)));
 
     // compile
     try!(rustc(pkg.get_root(), &src, &target, deps(pkg, pkgs)));
@@ -41,7 +43,7 @@ fn mk_target(target: &Path) -> io::IoResult<()> {
     io::fs::mkdir_recursive(target, io::UserRWX)
 }
 
-fn rustc(root: &Path, src: &Path, target: &Path, deps: &[core::Package]) -> CargoResult<()> {
+fn rustc(root: &Path, src: &Path, target: &Path, deps: &[core::Package]) -> CLIResult<()> {
     let mut args = Vec::new();
 
     build_base_args(&mut args, src, target);
@@ -50,7 +52,9 @@ fn rustc(root: &Path, src: &Path, target: &Path, deps: &[core::Package]) -> Carg
     try!(util::process("rustc")
         .cwd(root.clone())
         .args(args.as_slice())
-        .exec().to_cargo_error(format!("Couldn't execute rustc {}", args.connect(" ")), 1));
+        .exec()
+        .to_result(|err|
+            CLIError::new(format!("Couldn't execute rustc {}", args.connect(" ")), Some(err.to_str()), 1)));
 
     Ok(())
 }
@@ -78,7 +82,7 @@ fn deps(pkg: &core::Package, pkgs: &core::PackageSet) -> ~[core::Package] {
     pkgs.get_all(names).iter().map(|p| (*p).clone()).collect()
 }
 
-pub fn execute(_: NoFlags, manifest: core::Manifest) -> CargoResult<Option<core::Manifest>> {
+pub fn execute(_: NoFlags, manifest: core::Manifest) -> CLIResult<Option<core::Manifest>> {
     let core::Manifest { root, lib, bin, .. } = manifest;
 
     let (crate_type, out_dir) = if lib.len() > 0 {
@@ -86,7 +90,7 @@ pub fn execute(_: NoFlags, manifest: core::Manifest) -> CargoResult<Option<core:
     } else if bin.len() > 0 {
         ( "bin".to_owned(), bin[0].path )
     } else {
-        return Err(CargoError::new("bad manifest, no lib or bin specified".to_owned(), 1));
+        return Err(CLIError::new("bad manifest, no lib or bin specified", None, 1));
     };
 
     let root = Path::new(root);
@@ -111,12 +115,13 @@ pub fn execute(_: NoFlags, manifest: core::Manifest) -> CargoResult<Option<core:
     config.program = "rustc";
     config.args = args.as_slice();
 
-    let mut p = try!(Process::configure(config).to_cargo_error(format!("Could not start process: rustc {}", args.as_slice()), 1));
+    let mut p = try!(Process::configure(config).to_result(|err|
+        CLIError::new(format!("Could not start process: rustc {}", args.connect(" ")), Some(err.to_str()), 1)));
 
     let status = p.wait();
 
     if status != std::io::process::ExitStatus(0) {
-        fail!("Failed to execute")
+        return Err(CLIError::new(format!("Non-zero status code from rustc {}", args.connect(" ")), None, 1));
     }
 
     Ok(None)
diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
index 5f907459d..205b6df07 100644
--- a/src/cargo/sources/path.rs
+++ b/src/cargo/sources/path.rs
@@ -1,7 +1,9 @@
+use std::fmt;
+use std::fmt::{Show,Formatter};
 use core::{NameVer,Package};
 use core::source::Source;
 use core::manifest::Manifest;
-use CargoResult;
+use core::errors::{CargoResult,CargoCLIError,ToResult};
 use cargo_read_manifest = ops::cargo_read_manifest::read_manifest;
 
 pub struct PathSource {
@@ -14,6 +16,12 @@ impl PathSource {
     }
 }
 
+impl Show for PathSource {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f.buf, "the paths source")
+    }
+}
+
 impl Source for PathSource {
     fn update(&self) -> CargoResult<()> { Ok(()) }
 
@@ -42,5 +50,5 @@ impl Source for PathSource {
 
 fn read_manifest(path: &Path) -> CargoResult<Manifest> {
     let joined = path.join("Cargo.toml");
-    cargo_read_manifest(joined.as_str().unwrap())
+    cargo_read_manifest(joined.as_str().unwrap()).to_result(|err| CargoCLIError(err))
 }
diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
index c241199ac..358f6ca35 100644
--- a/src/cargo/util/config.rs
+++ b/src/cargo/util/config.rs
@@ -2,7 +2,7 @@ extern crate collections;
 extern crate serialize;
 extern crate toml;
 
-use super::super::{CargoResult,ToCargoError,CargoError};
+use core::errors::{CargoResult,CargoError,ToResult,Described,Other};
 use serialize::{Encodable,Encoder};
 use std::{io,fmt};
 
@@ -75,7 +75,8 @@ impl fmt::Show for ConfigValue {
 }
 
 pub fn get_config(pwd: Path, key: &str) -> CargoResult<ConfigValue> {
-    find_in_tree(&pwd, |file| extract_config(file, key)).to_cargo_error(format!("Config key not found: {}", key), 1)
+    find_in_tree(&pwd, |file| extract_config(file, key)).to_result(|_|
+        CargoError::internal(Described(format!("Config key not found: {}", key))))
 }
 
 pub fn all_configs(pwd: Path) -> CargoResult<collections::HashMap<~str, ConfigValue>> {
@@ -99,7 +100,7 @@ fn find_in_tree<T>(pwd: &Path, walk: |io::fs::File| -> CargoResult<T>) -> CargoR
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).to_cargo_error("".to_owned(), 1));
+            let file = try!(io::fs::File::open(&possible).to_result(|_| CargoError::internal(Other)));
             match walk(file) {
                 Ok(res) => return Ok(res),
                 _ => ()
@@ -109,7 +110,7 @@ fn find_in_tree<T>(pwd: &Path, walk: |io::fs::File| -> CargoResult<T>) -> CargoR
         if !current.pop() { break; }
     }
 
-    Err(CargoError::new("".to_owned(), 1))
+    Err(CargoError::internal(Other))
 }
 
 fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult<()> {
@@ -119,14 +120,14 @@ fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).to_cargo_error("".to_owned(), 1));
+            let file = try!(io::fs::File::open(&possible).to_result(|_| CargoError::internal(Other)));
             match walk(file) {
                 Err(_) => err = false,
                 _ => ()
             }
         }
 
-        if err { return Err(CargoError::new("".to_owned(), 1)); }
+        if err { return Err(CargoError::internal(Other)); }
         if !current.pop() { break; }
     }
 
@@ -134,25 +135,25 @@ fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult
 }
 
 fn extract_config(file: io::fs::File, key: &str) -> CargoResult<ConfigValue> {
-    let path = try!(file.path().as_str().to_cargo_error("".to_owned(), 1)).to_owned();
+    let path = try!(file.path().as_str().to_result(|_| CargoError::internal(Other))).to_owned();
     let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf).to_cargo_error("".to_owned(), 1));
-    let val = try!(root.lookup(key).to_cargo_error("".to_owned(), 1));
+    let root = try!(toml::parse_from_buffer(&mut buf).to_result(|_| CargoError::internal(Other)));
+    let val = try!(root.lookup(key).to_result(|_| CargoError::internal(Other)));
 
     let v = match val {
         &toml::String(ref val) => String(val.to_owned()),
         &toml::Array(ref val) => List(val.iter().map(|s: &toml::Value| s.to_str()).collect()),
-        _ => return Err(CargoError::new("".to_owned(), 1))
+        _ => return Err(CargoError::internal(Other))
     };
 
     Ok(ConfigValue{ value: v, path: vec!(path) })
 }
 
 fn extract_all_configs(file: io::fs::File, map: &mut collections::HashMap<~str, ConfigValue>) -> CargoResult<()> {
-    let path = try!(file.path().as_str().to_cargo_error("".to_owned(), 1)).to_owned();
+    let path = try!(file.path().as_str().to_result(|_| CargoError::internal(Other))).to_owned();
     let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf).to_cargo_error("".to_owned(), 1));
-    let table = try!(root.get_table().to_cargo_error("".to_owned(), 1));
+    let root = try!(toml::parse_from_buffer(&mut buf).to_result(|_| CargoError::internal(Other)));
+    let table = try!(root.get_table().to_result(|_| CargoError::internal(Other)));
 
     for (key, value) in table.iter() {
         match value {
@@ -173,11 +174,11 @@ fn extract_all_configs(file: io::fs::File, map: &mut collections::HashMap<~str,
 
 fn merge_array(existing: &mut ConfigValue, val: &[toml::Value], path: &str) -> CargoResult<()> {
     match existing.value {
-        String(_) => return Err(CargoError::new("".to_owned(), 1)),
+        String(_) => return Err(CargoError::internal(Other)),
         List(ref mut list) => {
             let new_list: Vec<CargoResult<~str>> = val.iter().map(|s: &toml::Value| toml_string(s)).collect();
             if new_list.iter().any(|v| v.is_err()) {
-                return Err(CargoError::new("".to_owned(), 1));
+                return Err(CargoError::internal(Other));
             } else {
                 let new_list: Vec<~str> = new_list.move_iter().map(|v| v.unwrap()).collect();
                 list.push_all(new_list.as_slice());
@@ -191,6 +192,6 @@ fn merge_array(existing: &mut ConfigValue, val: &[toml::Value], path: &str) -> C
 fn toml_string(val: &toml::Value) -> CargoResult<~str> {
     match val {
         &toml::String(ref str) => Ok(str.to_owned()),
-        _ => Err(CargoError::new("".to_owned(), 1))
+        _ => Err(CargoError::internal(Other))
     }
 }
diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs
index a428f825c..81fce8bd4 100644
--- a/src/cargo/util/important_paths.rs
+++ b/src/cargo/util/important_paths.rs
@@ -1,4 +1,4 @@
-use super::super::{CargoResult,CargoError};
+use core::errors::{CargoResult,CargoError,MissingManifest};
 
 pub fn find_project(pwd: Path, file: ~str) -> CargoResult<Path> {
     let mut current = pwd.clone();
@@ -11,5 +11,5 @@ pub fn find_project(pwd: Path, file: ~str) -> CargoResult<Path> {
         if !current.pop() { break; }
     }
 
-    Err(CargoError::new(format!("Could not find a Cargo manifest ({}) in your current directory or any parent directory", file), 1))
+    Err(CargoError::internal(MissingManifest(pwd, file)))
 }
diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs
index 3c1b808c3..cffd45798 100644
--- a/src/cargo/util/process_builder.rs
+++ b/src/cargo/util/process_builder.rs
@@ -3,8 +3,7 @@ use std::path::Path;
 use std::io;
 use std::io::process::{Process,ProcessConfig,ProcessOutput,InheritFd};
 use collections::HashMap;
-use ToCargoError;
-use CargoResult;
+use core::errors::{ToResult,CargoResult,CargoError,Described};
 
 #[deriving(Clone,Eq)]
 pub struct ProcessBuilder {
@@ -68,13 +67,16 @@ impl ProcessBuilder {
         config.args = self.args.as_slice();
         config.cwd = Some(&self.cwd);
 
-        let os_path = try!(os::getenv("PATH").to_cargo_error("Could not find the PATH environment variable".to_owned(), 1));
+        let os_path = try!(os::getenv("PATH").to_result(|_|
+            CargoError::internal(Described("Could not find the PATH environment variable".to_owned()))));
+
         let path = os_path + PATH_SEP + self.path.connect(PATH_SEP);
 
         let path = [("PATH".to_owned(), path)];
         config.env = Some(path.as_slice());
 
-        Process::configure(config).map(|mut ok| ok.wait_with_output()).to_cargo_error("Could not spawn process".to_owned(), 1)
+        Process::configure(config).map(|mut ok| ok.wait_with_output()).to_result(|_|
+            CargoError::internal(Described("Could not spawn process".to_owned())))
     }
 
     fn build_config<'a>(&'a self) -> io::IoResult<ProcessConfig<'a>> {